I am unsure if the POSIX path will work on windows - can someone with cpp experience give me some hints?

otherwise I think the PR is ready for review :)

Thanks @capital-G
Could you please rebase your PR onto develop to resolve conflicts in .gitignore?

Fixed the merge conflict by rebasing

Any update here? :)

How would that work in practice? Are there any caveats concerning the dependency?

Hey @gusano and @jrsurge , any kind of things thats blocking this from merging? Am happy to provide any infos or code that is missing!

How would that work in practice? Are there any caveats concerning the dependency?

You would need to re-compile the scss files to css files with a sass compiler if you change something on the main css files, the exact steps are documented in https://github.com/supercollider/supercollider/blob/1c3e690b9f47e97f15f32e6ac3cf5a7cf880db0c/HelpSource/themes/README.md

I think this dependency is reasonably weak, that is if in the future SASS seems like not a good idea any more, there are still the css.

BUMP

What is missing from a merge? I think this would be a great addition to 3.13 as I see many people using a dark skin but a bright documentation.

Hi all - we'd like to pull this in for 3.13.0 and are planning an RC for that after our meeting on Aug, 27, 2022. Anything else to add? Or can we close this?

Maybe we could add a github action which checks if the the repo-included compiled css matches the source code from the scss? This way we avoid any kind of modification in the css which is not reflected in the scss.

Wouldn't it be easier (also in maintenance) to add a big comment in the CSS file that informs future developers about this?

Thanks for the PR, apologies for the delay in review.

I have some quite big concerns about this PR as it stands:

1. It adds a silent dependency on an external tool to generate code. We won't always need to run it, but that means that we won't know that we need the generator until it's too late. This is not an ideal developer workflow.

2. The external tool has additional dependencies:

   • It can be acquired as a node.js utility, which requires developers to have node set up locally
   • Or it can be installed via homebrew or choco - homebrew is not an issue, but choco is both uncommon and unreliable.

3. There is no version requirement for the generator, so we will likely end up with different output depending on who last generated it.

4. In order to reduce the impact of the silent dependency, the solution has been to commit the generated code to the repository. This worries me because we will likely end up out of sync and potentially have to deal with rather large merge conflicts.

5. The expected way to run the generator is through a shell script. This means that developers on Windows have a much more difficult time. We must be mindful of cross-platform, especially with our tooling.

While I greatly appreciate the work that's gone in to this PR, taking all of this into account I respectfully recommend that this be closed until we can devise a well-supported and cross-platform approach in line with the project.

Thank you @jrsurge for the critical examination. Maybe a solution comes up here that is less intrusive.

I have some quite big concerns about this PR as it stands:

I completely agree with all of these issues.

I now provided a Docker Image (and instructions) which provides a reference building environment (with fixed versions) regardless of ones host machine. I also added a CI/CD pipeline which verifies the checked in CSS files based on the SCSS files every time, raising awareness if something in the building process starts to break, see https://github.com/supercollider/supercollider/runs/7921724963?check_suite_focus=true

The whole process is described @ https://github.com/supercollider/supercollider/blob/c68d134d93159a2075fab6da563171f952996397/HelpSource/themes/README.md

@jrsurge: Do you think this fixes the issues to an acceptable amount?

Wouldn't it be easier (also in maintenance) to add a big comment in the CSS file that informs future developers about this?

There is already a comment at the beginning of each CSS file but what I learnt from programming is that you should never trust yourself, so automatically running checks are a really convenient way for maintenance :)

Any feedback on the proposed changes? Does this seem acceptable or not, and if not, what could be an alternative?

Why this was this moved out of 3.13 / https://github.com/supercollider/supercollider/milestone/24 ? :/
It is really frustrating to build simple features that get on hold w/o any remarks what steps are blocking a merge. I must admit it seems to me that the SC dev community is not very welcoming to contributions other than bug fixes, at least it drags a lot of my motivation to contribute to SC.

@capital-G sorry that this was neglected, it is not about welcoming but about expertise. @jrsurge what is your view? Do you think the issues are alleviated or still too worrying? I can't really judge it very well.

Apologies for the delay in my response, I haven't been able to contribute recently.

While I can appreciate the frustration of having to wait for feedback and then having the PR moved out of scope for the next release before having that feedback, I'd like to take this as an opportunity to remind contributors that we are all volunteers and the last few years have been very difficult for everyone - it takes a lot of time and energy to write PRs, but also to review them. I am genuinely sorry that it has taken so long, and I'd ask for your patience and compassion.

In short, my recommendation to close this PR remains unchanged.

Ultimately, I think we need to go back to the initial issue and design the simplest thing possible.

This PR began with adding SASS to the project, which was identified as an issue in terms of dependencies - it was a hidden dependency in that it was only required sometimes, and not provided, and added even more dependencies: a SASS compiler, which itself added nodejs and all of its dependencies. In order to solve those dependency issues, another dependency was added (Docker) so that the other dependencies were abstracted and hidden away, adding a huge technology stack in the process.

The problem of theming the documentation is fundamentally one of replacing a fairly limited number of hex values inside a template and loading it at runtime. Both proposed solutions achieve this (in part), but with the current solution I find it very difficult to justify adding a full virtualisation/container technology stack to the project to replace strings in a text file.
In addition, while the current solution solves the problem for built-in schemes, it does not account for custom schemes that users can create via the IDE. I believe there is no iteration of this PR that would account for that as it is fundamentally the wrong design, given that it is effectively compile-time.

To clarify, I think the goal of themed documentation is a great one, and one that we should strive for, but I think the approach needs to be reconsidered, ideally in collaboration with other contributors.

As a recommendation for future PRs, if a PR has a particularly large technology or design change, like this one, it might be worth opening an RFC so that we can discuss the architecture/design more formally before someone goes and does a lot of work. It is not fun to close PRs for anyone involved - I don't want you to take the closing of this PR as any form of personal rejection, let's work together on these things!

Please feel free to send me a message on the SC slack if it'd be useful, I'm more than happy to talk about things!

@capital-G - apologies for not leaving a direct message as well about why it was moved out of the milestone. I should have done that. This was a difficult one for us to review and decide on given the dependencies added. We did discuss it during a dev meeting - if you would like to discuss more, please join us this Sunday (Nov 13, 2022). Thanks!

Thanks for your response @jrsurge - I think the focus on keeping SuperCollider maintainable is understandable but leads to more unmaintainable code in the long run. The current state of the CSS code is not in a good shape, making it effectively already unmaintainable to implement an easy feature/bug fix as applying themes to docs gets rejected - I don't see a cleaner approach then the proposed one as it also cleans up the CSS files. SASS has become a new standard in web projects and while is understandable that this causes further complexity in the build chain this complexity can simply be mitigated nowadays with tools such as docker and/or pre-commit (see #5804 as well - effectively we could replace any source code managing dependency via docker) and a CI pipeline.

By not making use of modern tools for development SuperCollider tends to a conservatory, LTS state, suggesting that the current state is settled which also undermines new features.

In addition, while the current solution solves the problem for built-in schemes, it does not account for custom schemes that users can create via the IDE.

I think it is adaptable by writing a reflecting SASS file and compiling a CSS out of it - adding a new official theme is then as easy as adding it here and providing a SASS file.

I decided against the option to mangle with the QtBrowser or via JS because

• the QtIDE seems abandoned to me, therefore I think its best to introduce changes at a more general adaptable level so once there is a migration to something new we can re-use as many features as possible
• adding JS to manage this adds new runtime complexity which then indeed needs to be maintained and tested - IMO a stateless build dependency via Docker at compile time is far less maintenance than code which runs at runtime - in this case we also depend on the JS engine of Qt which I am unsure of its featureset - especially across platforms.

As a recommendation for future PRs, if a PR has a particularly large technology or design change, like this one, it might be worth opening an RFC so that we can discuss the architecture/design more formally before someone goes and does a lot of work.

I think the RFC workflow is way to bureaucratic and I don't understand its purpose over creating an (meta-)issue (at least for a project of the size/userbase of SC).
But when proposing and discussing issues it also tends to a halt - to overcome this halt my motivation is often to simply implement it. IMO the major time consuming task is not the implementation but discussing them - which is of course necessary. Implementing and rewriting the CSS part was like 6 hours of work, the discussion around it is running since around 1 1/2 years and took already more than 6 hours.

I must admit when comparing the rate of discussion/implementation in SC to other projects I contribute to SC has by far the highest ratio - and also the success rate of a merge in other projects is nearly 100%, at SC its on the other end. And in the end this is not my style of working. I think effectively that I don't have any motivation left to contribute to SC b/c of its more conservative tendency, there is this famous quote by Torvalds who says choose a OS project based upon the environment you like. Its not solely this issue but more a general tendency.

Sorry to hear this @capital-G, I would have enjoyed working with you in the way Torwalds meant it. The difficulty with a project like SC is not that it is conservative, but that it is careful, because it knows where its limits are.

In SC, often it is simply that people are responsible at least for the code they wrote themselves. Depending on the complexity of the code, once somone leaves, it may mean it adds technological debt (even if it was supposed to make things better).

So actually, my question would have been with respect to the deeper of your suggestions: how long would you be around in person to support them?

@telephon I meant conservative not in any negative way, Debian is also conservative and provides a much needed stability with this approach. Yet I would like SC to be more fluid and streamlined than in its current state and I think this is where there is a divergence which will lead to frustration at least on my end.

In the end I am convinced that the SC dev team knows how to maintain SC as it did so longer than I did.

So actually, my question would have been with respect to the deeper of your suggestions: how long would you be around in person to support them?

In my experience one can never be too sure or promise how long contributors will stay along, and it is therefore important that the footprint is preferred to be static, stateless and that any checks run automatically at least in CI and are documented, allowing to be aware of any conflicts as early as possible.
For my case I decided to stop contributing any new features/fixes, yet I will still be around for supporting any code that I am responsible for (as long as the dev environment is easy to setup, but see e.g. #5804 for discussion), although my footprint is rather limited.

Hey @jrsurge - in the meantime I have spend some more time with CSS in some projects and I think the proposed changes could be made with only using CSS and variables, making a SASS compiler obsolete.

Do you think this could get merged if it is CSS only? Then I would re-visit this PR and change it to CSS only.

I can't quite see what effect this PR would have, but I wanted to highlight an important workflow: to make the SuperCollider documentation readable in Visual Studio Code, I need to provide quite a large number of CSS overrides that connect selectors to variables with theme colors specified by VSCode. Without this, the documentation is somewhere between ugly and completely unusable. I'm currently doing this via the frontend.css file:
https://github.com/scztt/vscode-supercollider/blob/develop/src/commands/help/frontend.css

This isn't the BEST way to achieve something like this, but I'm pretty far along and things are improving so I think this will be an acceptable (if not perfect) solution. I just wanted to make sure that any change to the css here preserves the ability to supply IDE-specific overrides using a similar mechanism.

Do you think this could get merged if it is CSS only? Then I would re-visit this PR and change it to CSS only.

Yes, I think this would be very good! As @scztt mentioned, IDE-specific overrides should be possible.